fix(otel): end recording spans on non-terminal by ayushiahjolia · Pull Request #696 · aws/aws-durable-execution-sdk-python

ayushiahjolia · 2026-08-31T18:51:05Z

Issue #, if available:
#642

Description of changes:

Workflow span: created+ended once, on the terminal invocation (both plugins).
Attempt span: created+ended in the same invocation (both plugins).
Operation span: invocation plugin → one segment per invocation (fresh ID + link across invocations); execution plugin → one span total, created+ended in the invocation where the operation terminates (held as a non-recording placeholder while it spans invocations).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Sign in to view

+        with self._lock:
+            if info.operation_id in self._ended_operation_ids:
+                return
+            self._ended_operation_ids.add(info.operation_id)


Sign in to view

+        self._pop_span(info.operation_id)
        parent = self._resolve_parent(info.parent_id)
-        self._start_span(
+        span = self._start_span(


Sign in to view

+        with self._lock:
+            keys = list(reversed(self._operation_spans))


github-actions · 2026-09-03T00:31:12Z

+        with self._lock:
+            if info.operation_id in self._ended_operation_ids:
+                return
+            self._ended_operation_ids.add(info.operation_id)


Codex AI review · Finding arf_v1_2pna33ayjyhsqx2ehym77e3n7t

[P1] Do not re-export terminal contexts during replay. This set is reset after every invocation, so ReplayChildren contexts calling on_child_context_end(..., is_replayed=True) export the same deterministic span again on each replay. Collectors may overwrite or reject the original span. Skip replayed terminal callbacks or otherwise make export uniqueness execution-wide, and add a multi-invocation ReplayChildren test asserting one export.

github-actions · 2026-09-03T00:31:13Z

+        self._pop_span(info.operation_id)
        parent = self._resolve_parent(info.parent_id)
-        self._start_span(
+        span = self._start_span(


Codex AI review · Finding arf_v1_5yc45ob4ohc33i7vrvqz7pezbu

[P2] Preserve a start timestamp for checkpointless contexts. on_child_context_end() captures end_time but supplies start_time=None, so this span starts at materialization time and is then ended using an earlier timestamp. Virtual and ReplayChildren context spans can consequently have negative durations. Store the user-function start time alongside the placeholder or include it in the end hook, and test both paths.

github-actions · 2026-09-03T00:31:14Z

+        with self._lock:
+            keys = list(reversed(self._operation_spans))


Codex AI review · Finding arf_v1_dv5gcy7ci2c3j7gxu7vdwsejwy

[P2] Close attempts overwritten by in-process re-entry. This sweep can see only the latest span stored for each key. If an incomplete STEP re-enters the same attempt, _start_span() replaces the first recording span; ending the replacement then leaves the original unreachable and unended. End or retain the existing attempt before replacement, and extend the re-entry test to assert the original span is no longer recording.

github-actions · 2026-09-03T00:31:16Z

Codex AI review

Found three telemetry correctness issues, including replay-time duplicate deterministic spans. Review was read-only; tests were not executed as requested.

Reviewed commit 216222aaac4c038d1bb7fc8e2e010e80c8e2885c. Workflow run

github-actions · 2026-09-03T00:34:46Z

+    def _end_open_recording_spans(self) -> None:
+        """End recording user-function spans left open by a suspended operation.
+
+        Operation placeholders are non-recording and export their span from
+        on_operation_end, so they are skipped. Reverse order keeps each child
+        contained within its parent; the invocation span is ended by the caller.
+        """
+        with self._lock:
+            keys = list(reversed(self._operation_spans))
+        for key in keys:
+            if key == _INVOCATION_KEY:
+                continue
+            span = self._get_span(key)
+            if span is None or not span.is_recording():
+                continue
+            popped = self._pop_span(key)
+            if popped is not None:
+                popped.end()


Claude AI review · Finding arf_v1_d3v4nkts2wbw4vfdae6gv4poj6

_end_open_recording_spans() force-closes any recording span still open at invocation end (the realistic case is a STEP attempt whose outcome was INCOMPLETE, e.g. an in-process retry that didn't finish before the invocation suspended) by calling popped.end() with no status or attribute set. The exported span therefore has StatusCode.UNSET and none of the durable.attempt.* attributes that a normal completed attempt gets from on_user_function_end, so in a trace viewer it is indistinguishable from a short, uneventful successful attempt — hiding the fact that it was actually truncated because the invocation ended mid-attempt.

Consider setting an explicit marker before ending, e.g. popped.set_attribute("durable.span.truncated_at_invocation_boundary", True) (and/or leaving status UNSET is fine, but the attribute gives downstream consumers a way to distinguish this from a genuinely completed, short attempt).

github-actions · 2026-09-03T00:34:48Z

Claude AI review

This PR replaces the previous "drop-without-ending" handling of open operation spans in ExecutionOtelPlugin with a non-recording-placeholder design: every operation/child-context/Workflow span is held as a deterministic NonRecordingSpan while open, and the single real (recording) span is created and immediately ended exactly once — either in on_operation_end for operations, or in the new _export_workflow_span for the Workflow span on a terminal invocation. InvocationOtelPlugin gets the analogous Workflow-placeholder treatment plus a shared _resolved_trace_state() helper that now correctly preserves a same-trace ambient tracestate for operation-link/placeholder contexts (previously it always used the ancestor's always-empty TraceState()).

I traced the full lifecycle (start/replay/suspend/resume/end, cross-invocation stitching, CONTEXT vs STEP vs WAIT/CALLBACK, duplicate on_operation_end calls, and parent resolution for nested contexts) against the SDK's checkpoint/plugin-dispatch semantics in plugin.py and state.py. The redesign correctly resolves the previously-flagged duplicate-deterministic-span-ID risk (open operations are now placeholders, never separately re-exported), the new _ended_operation_ids guard is properly locked and reset per invocation, and _end_open_recording_spans correctly skips non-recording placeholders while safely closing any leftover recording spans (i.e., in-progress STEP attempt spans) so none are silently abandoned. Parent resolution at on_operation_end time (rather than at start time, as before) is safe given the SDK's synchronous checkpoint-then-unblock model, which guarantees a child's completion hook always runs before its parent context's own completion checkpoint can even be enqueued. New unit and e2e tests exercise suspend/resume, duplicate-end, and cross-plugin tracestate propagation adequately.

One minor completeness gap remains (see inline comment): spans forcibly closed by _end_open_recording_spans (e.g. an in-flight STEP attempt abandoned when the invocation boundary hits) are ended with no status/attribute indicating they were truncated, so they can look like normal short-lived successful spans in a trace viewer. No other correctness, typing, or public-API issues were found; residual risk is limited to this cosmetic/observability nuance.

Reviewed commit 216222aaac4c038d1bb7fc8e2e010e80c8e2885c. Workflow run

ayushiahjolia mentioned this pull request Aug 31, 2026

fix(otel): end recording spans on non-terminal invocations #671

Closed

ayushiahjolia force-pushed the otel-shared-trace-pr671-end-spans branch from 95550ea to 0e1fd55 Compare September 1, 2026 18:37

ayushiahjolia marked this pull request as ready for review September 1, 2026 18:48

ayushiahjolia temporarily deployed to ai-pr-review-runtime September 1, 2026 18:49 — with GitHub Actions Inactive

ayushiahjolia requested a review from zhongkechen September 1, 2026 18:53

ayushiahjolia had a problem deploying to ai-pr-review-runtime September 1, 2026 19:11 — with GitHub Actions Error

ayushiahjolia had a problem deploying to ai-pr-review-runtime September 1, 2026 19:14 — with GitHub Actions Error

ayushiahjolia had a problem deploying to ai-pr-review-runtime September 1, 2026 19:40 — with GitHub Actions Failure

ayushiahjolia force-pushed the otel-shared-trace-pr671-end-spans branch from 0e1fd55 to 51018c3 Compare September 1, 2026 19:42

Base automatically changed from otel-shared-trace-pr5-docs to main September 1, 2026 19:42

ayushiahjolia force-pushed the otel-shared-trace-pr671-end-spans branch from 51018c3 to 610690e Compare September 1, 2026 19:42

ayushiahjolia temporarily deployed to ai-pr-review-runtime September 1, 2026 20:04 — with GitHub Actions Inactive

github-actions Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py

github-actions Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py Outdated

This comment has been minimized.

Sign in to view

github-actions Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py

This comment has been minimized.

Sign in to view

ayushiahjolia marked this pull request as draft September 1, 2026 21:12

ayushiahjolia force-pushed the otel-shared-trace-pr671-end-spans branch from af13837 to 3bbcad6 Compare September 1, 2026 21:37

ayushiahjolia marked this pull request as ready for review September 1, 2026 21:45

ayushiahjolia temporarily deployed to ai-pr-review-runtime September 1, 2026 21:46 — with GitHub Actions Inactive

ayushiahjolia had a problem deploying to ai-pr-review-runtime September 1, 2026 22:00 — with GitHub Actions Error

ayushiahjolia temporarily deployed to ai-pr-review-runtime September 1, 2026 22:05 — with GitHub Actions Inactive

zhongkechen reviewed Sep 1, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py

zhongkechen previously approved these changes Sep 1, 2026

View reviewed changes

github-actions Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py

This comment has been minimized.

Sign in to view

ayushiahjolia force-pushed the otel-shared-trace-pr671-end-spans branch from 3bbcad6 to ac01aaf Compare September 1, 2026 22:49

ayushiahjolia had a problem deploying to ai-pr-review-runtime September 1, 2026 23:01 — with GitHub Actions Failure

ayushiahjolia dismissed zhongkechen’s stale review via 210ce63 September 1, 2026 23:06

ayushiahjolia force-pushed the otel-shared-trace-pr671-end-spans branch from ac01aaf to 210ce63 Compare September 1, 2026 23:06

ayushiahjolia had a problem deploying to ai-pr-review-runtime September 1, 2026 23:16 — with GitHub Actions Failure

ayushiahjolia temporarily deployed to ai-pr-review-runtime September 1, 2026 23:16 — with GitHub Actions Inactive

github-actions Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py

This comment has been minimized.

Sign in to view

ayushiahjolia force-pushed the otel-shared-trace-pr671-end-spans branch from 210ce63 to 1d65bfb Compare September 2, 2026 14:29

ayushiahjolia temporarily deployed to ai-pr-review-runtime September 2, 2026 14:29 — with GitHub Actions Inactive

ayushiahjolia had a problem deploying to ai-pr-review-runtime September 2, 2026 14:29 — with GitHub Actions Failure

github-actions Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py

self._pop_span(info.operation_id)

parent = self._resolve_parent(info.parent_id)

self._start_span(

span = self._start_span(

This comment was marked as outdated.

Sign in to view

github-actions Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread ...able-execution-sdk-python-otel/src/aws_durable_execution_sdk_python_otel/execution_plugin.py

Comment on lines +511 to +512

with self._lock:

keys = list(reversed(self._operation_spans))

This comment was marked as outdated.

Sign in to view

This comment has been minimized.

Sign in to view

fix(otel): end recording spans on non-terminal invocations

216222a

zhongkechen force-pushed the otel-shared-trace-pr671-end-spans branch from 1d65bfb to 216222a Compare September 2, 2026 22:14

zhongkechen temporarily deployed to ai-pr-review-runtime September 2, 2026 23:04 — with GitHub Actions Inactive

zhongkechen deployed to ai-pr-review-runtime September 2, 2026 23:04 — with GitHub Actions Active

github-actions Bot reviewed Sep 3, 2026

View reviewed changes

		with self._lock:
		keys = list(reversed(self._operation_spans))

Uh oh!

Conversation

ayushiahjolia commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment has been minimized.

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 3, 2026

Codex AI review

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 3, 2026

Claude AI review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ayushiahjolia commented Aug 31, 2026 •

edited

Loading